feat(hooks): guard the shared process table — kill only a PID you recorded - #16843
Conversation
…orded Worktree isolation gives each agent its own checkout, index and HEAD; it gives it nothing over the process table, which is one table per container. A name-matched kill reaches whatever a parallel agent is running under that name, and it exits 0 either way — the agent that fired it observes nothing, the agent whose run died cannot attribute it, and the loss is recorded as a flake. The same shape as guard-shared-stash.sh, deliberately: hook + same-named .selftest.sh + an OS_ALLOW_* escape + the Bash PreToolUse registration, with the AGENTS.md line and the CLAUDE.md inline beside the stash rule they generalise. Swept first, so the rule covers the class rather than one spelling: the tree carries no pkill, no killall, no `xargs kill`, no `ps | grep | kill` and no `fuser -k`. Every live kill site is PID-scoped — `kill "$SERVER_PID"`, `kill $(lsof -ti tcp:PORT)`, `pgrep -s`/`pgrep -P` in publish-smoke.sh and gen-sdui-manifest.sh — and the self-test pins all of them ALLOWED alongside the blocked shapes, including the card's own specimen. Neither ceiling moves: AGENTS.md 1067 -> 1068 (its one line of headroom) and CLAUDE.md stays at 41, the stash section absorbing the second rule inside its existing six lines. The mechanism, the class and the escape live in the hook header, which carries no ceiling — the ratchet's own prescribed remedy. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P58euzUXCVJNwmhuPC9DXY
… unpinned Second ablation leg: replacing check_pkill's `[ "$pidscoped" -eq 1 ] && return 0` with an unconditional allow left the self-test green at 68/0. Bare `pkill` is the only input that reaches that gate — every other spelling returns earlier — so the branch had no case standing on it. It does now. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P58euzUXCVJNwmhuPC9DXY
维护者速读(终稿)席位复核记录:#16182 上的 ACCEPT 评论(skills 席,hotlong,session 改了什么五个文件。新增 PreToolUse 钩子 为什么改容器只有一张进程表,worktree 隔离对它无效; 风险与代价(含回滚)风险是误拦:对 席位意见同意合并。席位对照树逐项核过:分支 一处派发假设被 dev 依 os-dev 标准合同证伪:席位派发令写了「可用折并(fold)付行」,而 os-dev.md 规定行棘轮唯一合法货币是删除内容、折并不得为新内容买行;dev 据此拒绝折并、只用现有余量——做得对,记为席位错误。同一条也说明 #16516 决策卡的选项 B 应读作「把重复陈述折成指针(删除)」,不是重排。 你要做的受管面( 是 Generated by Claude Code |
Fixes #16182
Worktree isolation gives each agent its own checkout, index and HEAD. It gives it nothing
over the process table: there is one per container, so a name-matched kill reaches whatever a
parallel agent happens to be running under that name — and it reports success either way.
The agent that fired it observes nothing; the agent whose run died sees a killed process and a
truncated log with no signal tying it back, and re-runs blaming a flake. That is the same
property that makes the
git stashrule a rule rather than advice, which is why this isguard-shared-stash.sh's shape and not a sixth one.1. The sweep — population fixed BEFORE the rule (triage's⚠️ )
Swept over all tracked files at
d4401f75b:pkill,killall,killall5,taskkill,fuser -k,xargs kill,pgrep, andps … | grep …pipelines.pkill(any form).claude/agents/os-dev.mdL92,skills/objectstack-pm-dispatch/rules/dev-template.mdL64 — both already state the rule)killall/killall5/taskkillxargs kill/fuser -kps … | grep …pipelinepgrep -s SID/pgrep -P PIDscripts/gen-sdui-manifest.shL71/L76,scripts/publish-smoke.shL154,packages/spec/scripts/gen-sdui-manifest-cleanup.test.tsL104/L112/L149kill PID/kill -0 PID/kill $(lsof -ti tcp:PORT)AGENTS.mdL144 and L527 and.github/workflows/scaffold-e2e.ymlSo the class the rule names is selection by NAME, not "the
killverb": every live killsite in the tree is PID-scoped and stays green. Nothing in the tree had to be rewritten, and
the stop-condition (a legitimate name-pattern kill that cannot be rewritten PID-scoped) did
not fire.
2. The rule the guard enforces
Positive form first, because a rule that only forbids gets routed into another spelling:
Blocked —
pkillwith any pattern operand or any unrecognised option (with or without-f);killallin every form but--help/-l/-V; apgreppattern substituted orpiped into a kill;
pspiped throughgrepin a command that also kills.Allowed —
kill/kill -0/kill -9on a pid, a job spec or your own group;kill $(lsof -ti tcp:PORT);pgrep -P/pgrep -seven piped into a kill;pkill -P/pkill -swith no pattern operand (add a pattern and it is blocked again); every read(
pgrep -f PATTERN,ps aux | grep name) — nothing dies, so nothing is blocked.Fail-CLOSED once the command word is confidently
pkill/killall(an option it does notrecognise is refused, not waved through), fail-OPEN on anything it cannot parse — the
guard-shared-stash.shcontract, exit 0 allow / exit 2 block. Escape:OS_ALLOW_PROCESS_KILL=1.Two boundaries are stated in the header rather than left to be rediscovered: wrapped
invocations (
bash -c, a script file) are not caught, and the pgrep/ps coupling iswhole-command rather than pipeline-exact.
3. The two governed lines, before and after
AGENTS.md— one line, appended to Multi-agent working discipline item 8, the existing"never stop someone else's server" rule this generalises. Nothing removed:
CLAUDE.md— the ⛔ stash section extended in place, line count unchanged. Before:After (same 6 lines, every clause of the old text still present):
The intro's hard-coded "the four rules" became "the rules" (one line, one word deleted):
the section now carries two rules and two hooks, so the count was about to become false.
4. Line budgets — measured first, no ceiling touched
pnpm check:pm-skill-ratchetat089173a8a, exit 0, its own verdict lines:AGENTS.md1067 → 1068 spends its one line of headroom.CLAUDE.mdstays at 41: the newcontent is absorbed into the stash section's existing six lines, which carried 166 bytes of
slack under the 120-byte cap. No ceiling is raised, no ruled clause deleted, and no line was
bought by re-wrapping — the mechanism, the class and the escape live in the hook header,
which carries no ceiling. That is the ratchet header's own prescribed remedy: "pays its way
by moving narrative out … instead of raising the roof."
AGENTS.mdgets one dense line andCLAUDE.mdone clause, so the failure mode ("reports success either way") is spelled out onlyin
CLAUDE.mdand the hook header, not inAGENTS.md. Buying a secondAGENTS.mdline needsa maintainer ruling, and this PR does not take one.
5. Tests
.claude/hooks/guard-process-kill.selftest.sh— 69 passed, 0 failed, exit 0. Discoveredautomatically by
lint.yml's hook self-test collector (it globs.claude/hooks/**/*.selftest.sh),so no workflow edit. All five sibling self-tests still pass: governed-enqueue 52/0,
main-checkout-bash 130/0, main-checkout 120/0, shared-stash 53/0, tree-enum 38/0.
Ablation — two legs, each proved to land on disk and each restored by blob hash:
is_name_selector's*) return 0(a bare operand IS the name pattern) →*) i=$((i + 1))allowcheck_pkill's[ "$pidscoped" -eq 1 ] && return 0→ unconditional allowexpects: on its first run it left the self-test green at 68/0. Bare
pkillis the onlyinput that reaches that gate — every other spelling returns earlier — so the branch had no case
standing on it. The pin was added (commit 2,
expect block 'pkill') and the leg then reds. Theablation found the hole instead of shipping it.
Restore evidence per leg: anchor line count 1 → 0 with the injected line present (so the edit
was not a no-op), mutated blob
8acbd348…/f8d384ff…≠ HEAD blobc59489df…, restored blobequals
c59489df…, andgit diff HEADnames no path. The mutation script carries atrap … EXIT INT TERMon absolute paths and restores withgit checkout HEAD -- PATH, never abare checkout.
6. Gates
node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstackre-derived inthe worktree after the files existed and after merging
origin/main(acf4d3833): 18families. All 18 run at
089173a8a, every exit code captured by redirect before any pipe, allexit 0. Reconciliation with
--ran:pnpm --filter @objectstack/lint run check:doc-formula-expressionsfirst returned exit 3 —PREREQUISITE NOT MET (unbuilt
@objectstack/formula/@objectstack/lint), which is not afinding; after
turbo run buildfor those two packages under the shared verify lock it is exit 0.node scripts/pm/check-governed-merges.mjs --test …→ exit 3, GOVERNED (5 of 5 paths hitthe register). Draft PR, human merge, no ready-flip, no enqueue, no auto-merge.
ESLint narrowing, three readings together: the universe is
eslint --no-inline-configover thediff's own paths and it accepted all five (they are inside the configured universe);
--format jsonreports 5 files linted, 0 errors; andeslint.config.mjsL328 states theconfig has "no
parserOptions.project, no typed@typescript-eslintrules", so nothing inthis diff can move a verdict on an untouched file. Measured at
089173a8a, the final commit.7.
skip-changeset— measured, not assumedOf the 70 published packages carrying a
files[]array, zero nameAGENTS.md,CLAUDE.mdor.claudein it (positive control:packages/spec'sfiles[]reads back as["dist","json-schema","liveness","prompts","llms.txt","README.md","src/**/*.zod.ts",…], so thescan is reading real arrays). Nothing published ships. Label applied.
验收备注
Out-of-scope observations, filed nowhere and deliberately:
skills/objectstack-pm-dispatch/rules/dev-template.mdL64-66 already states this rule inEnglish for dispatched devs, and
.claude/agents/os-dev.mdL92 states it in Chinese. Both arenow backed by a hook rather than by convention alone. Neither file is touched — the published
skills catalogue is untouched in full, and nothing here needs a twin there. Successor: any PR
that next revises the dev template. noted, not filed.
kill "$PID" && pgrep -f nodeblocks though the twohalves are unrelated. Pipeline-exact coupling needs separator bookkeeping the splitter does not
carry; the over-block errs toward the rule and has a one-variable way out. Stated in the hook
header as a known boundary. noted, not filed — successor: whoever hits it.
.claude/hooks/and.claude/settings.jsoncarry no line ceiling by design(
check-skill-line-ratchet.mjssays so explicitly), which is what makes "move the narrativeinto the hook header" a real payment rather than a dodge. noted, not filed.
维护者速读(草稿)
改了什么 —— 新增一个 PreToolUse 钩子
guard-process-kill.sh(带同名.selftest.sh、OS_ALLOW_PROCESS_KILL=1覆盖开关、在.claude/settings.json的Bash匹配器里注册),它拦截"按名字杀进程"这一类命令,放行所有"按你自己记下的 PID 杀"的写法。配套
AGENTS.md加一行、CLAUDE.md把原来的 ⛔ stash 小节就地扩成"stash 栈与进程表都不受 worktree 隔离"。为什么改 —— 容器里只有一张进程表。一次
pkill -f会打到隔壁 agent 正在跑的门禁,而且两边都看不见:开枪的人拿到 exit 0,中枪的人只看到进程被杀、日志被截断,没有任何线索指向邻居,
于是当成 flake 重跑。这种既不被肇事者察觉、也不被受害者归因的故障不会随经验减少。卡片来自一次
自报:PR #16120 的实现席在自己已经全绿的交付报告里主动记下这次滑坡,当时同容器有四个 agent 存活。
风险与代价(含回滚) —— 风险是误拦:钩子对⚠️ 行数天花板一格不动,
pkill/killall是 fail-closed(不认识的选项一律拦),且 pgrep/ps 与 kill 的耦合判断是"整条命令"而非"同一管道",所以
kill "$PID" && pgrep -f node这种不相干的组合也会被拦。代价方向是刻意选的:宁可偶尔误拦一条,也不放过一次跨 agent 的误杀,
出口是
OS_ALLOW_PROCESS_KILL=1。自测把仓库里现存的全部 PID-scoped 写法都钉成"必须放行",所以现有脚本不会变红。回滚成本极低:钩子只要从
.claude/settings.json摘掉即刻失效,两个文档改动是纯文本、可单独 revert。
AGENTS.md用掉了它仅剩的一行余量,下一张动
AGENTS.md的卡(#16814)将从 0 余量起步。席位意见 ——
你要做的 —— 请只看两处:①
CLAUDE.md那 6 行的新措辞是否仍然把 stash 规则说清楚了(合并两条规则是为了不加第五个小节、不抬天花板;若你认为进程表值得单独一节,那需要一次抬 ceiling 的裁决);
②
AGENTS.md新增的那一行是否放在了对的位置(挂在"别停别人的服务器"那条之后,而不是挂在 stash 段旁)。其余是机械可核的:门禁 18/18 全绿、消融两条腿、天花板 0 余量未抬。治理面 ⇒ 请人工合并,
⛔ 本席不翻 ready、不入队、不挂 auto-merge。
Generated by Claude Code